Conform to nitro's DataAvailabilityProvider interface #1

afkbyte · 2024-04-06T19:44:47Z

todo: test

…er interface

ethenotethan

This is cool - mostly stylistic and knits. Have a few questions and change requests 🚀

ethenotethan · 2024-04-25T21:52:27Z

arbnode/batch_poster.go

 	use4844 bool,
+	useEigenDA bool,


nit: feel like an enum (e.g, DataSource) would be the best representation for this

ethenotethan · 2024-04-25T21:54:21Z

arbnode/batch_poster.go

-	if use4844 {
+
+	if useEigenDA {
+		calldata, err = method.Inputs.Pack(


we should prolly be checking this error

ethenotethan · 2024-04-25T21:55:36Z

arbnode/batch_poster.go

 		if err != nil {
 			if config.DisableEigenDAFallbackStoreDataOnChain {
 				log.Warn("Falling back to storing data on chain", "err", err)
 				return false, errors.New("unable to post batch to EigenDA and fallback storing data on chain is disabled")
 			}
 		}

-		pointer, err := b.eigenDAWriter.Serialize(daRef)
+		pointer, err := b.eigenDAWriter.Serialize(blobID)


why not?

Suggested change

pointer, err := b.eigenDAWriter.Serialize(blobID)

sequencerMsg, err = b.eigenDAWriter.Serialize(blobID)

ethenotethan · 2024-04-26T07:10:15Z

arbnode/batch_poster.go

 		sequencerMsg = pointer
 	}

-	data, kzgBlobs, err := b.encodeAddBatch(new(big.Int).SetUint64(batchPosition.NextSeqNum), batchPosition.MessageCount, b.building.msgCount, sequencerMsg, b.building.segments.delayedMsg, b.building.use4844)
+	data, kzgBlobs, err := b.encodeAddBatch(new(big.Int).SetUint64(batchPosition.NextSeqNum), batchPosition.MessageCount, b.building.msgCount, sequencerMsg, b.building.segments.delayedMsg, b.building.use4844, true, blobInfo)


shouldn't we be binding this boolean as a config value to the building type?

ethenotethan · 2024-04-26T07:11:10Z

arbnode/sequencer_inbox.go

+
+	// in the case of eigenDA the serialized data looks like this
+	// [0-40] Header Vals
+	// [40-41]eigenDA header flag


nit

Suggested change

// [40-41]eigenDA header flag

// [40-41] eigenDA header flag

ethenotethan · 2024-04-26T07:25:01Z

das/eigenda/eigenda.go

@@ -211,3 +339,139 @@ func RecoverPayloadFromEigenDABatch(ctx context.Context,
 	}
 	return data, nil
 }
+
+// calldata layout of sequencer msg


NIT - could be more readable to map this doc string into the function directly

ethenotethan · 2024-04-26T07:27:55Z

das/eigenda/eigenda.go

+
+}
+
+func convertCalldataToInt(calldata []byte) (int, error) {


nit - bytestoInt. Calldata implies that its being fed the calldata for the whole tx

ethenotethan · 2024-04-26T07:29:06Z

das/eigenda/eigenda.go

+	num := new(big.Int).SetBytes(calldata)
+
+	if num.IsInt64() {
+		return int(num.Uint64()), nil


what's the reason for this casting? Could we ever receive a negative value input in this function?

ethenotethan · 2024-04-26T07:31:13Z

das/eigenda/eigenda_test.go

+	"testing"
+)
+
+func TestParseSequencerMsg(t *testing.T) {


nit - maybe we could leverage the existing nitro test helpers for this to make it more idiomatic with the repo design

ethenotethan · 2024-04-26T07:32:11Z

util/eigenda/eigenda_blobs.go

@@ -0,0 +1,131 @@
+package eigenda_blobs
+


not at a point yet where I could comfortably review this section. Would it be worth getting one of our kzg SMEs to take a look?

Tristan-Wilson · 2024-04-30T15:33:14Z

Hi, we are doing some more work on further changes to the DA interface in Nitro and would welcome any feedback OffchainLabs/nitro#2157

ethenotethan · 2024-05-06T06:15:49Z

arbnode/batch_poster.go

+			new(big.Int).SetUint64(uint64(prevMsgNum)),
+			new(big.Int).SetUint64(uint64(newMsgNum)),
+		)
+		kzgBlobs, err = blobs.EncodeBlobs(l2MessageData)


where do we override this? If not mistaken this is using the same encoding for 4844 blobs

prpeh · 2024-06-03T14:35:41Z

go.mod

-go 1.20
+go 1.21
+
+toolchain go1.21.0


This make error build on my docker local build

RUN go mod download go: errors parsing go.mod: /workspace/go.mod:5: unknown directive: toolchain

If remove the toolchain go1.21.0 then an error on building smart contract happen as below

11.25 $ hardhat compile 22.44 Error HH411: The library eigenda, imported from src/bridge/ISequencerInbox.sol, is not installed. Try installing it using npm. 22.44 HardhatError: HH411: The library eigenda, imported from src/bridge/ISequencerInbox.sol, is not installed. Try installing it using npm. 22.44 at Resolver.resolveImport (/workspace/contracts/node_modules/hardhat/src/internal/solidity/resolver.ts:254:15) 22.44 at async /workspace/contracts/node_modules/hardhat/src/internal/solidity/dependencyGraph.ts:188:28 22.44 at async Promise.all (index 3) 22.44 at async DependencyGraph._addDependenciesFrom (/workspace/contracts/node_modules/hardhat/src/internal/solidity/dependencyGraph.ts:186:5) 22.44 at async Promise.all (index 17) 22.44 at async Function.createFromResolvedFiles (/workspace/contracts/node_modules/hardhat/src/internal/solidity/dependencyGraph.ts:15:5) 22.44 at async Environment._runTaskDefinition (/workspace/contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:333:14) 22.44 at async Environment.run (/workspace/contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:166:14) 22.44 at async SimpleTaskDefinition.action (/workspace/contracts/node_modules/hardhat/src/builtin-tasks/compile.ts:1346:58) 22.44 at async Environment._runTaskDefinition (/workspace/contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:333:14) 22.44 22.44 Caused by: HardhatError: HH401: Library eigenda is not installed. 22.44 at Resolver._resolveLibrarySourceName (/workspace/contracts/node_modules/hardhat/src/internal/solidity/resolver.ts:322:15) 22.44 at Resolver.resolveSourceName (/workspace/contracts/node_modules/hardhat/src/internal/solidity/resolver.ts:103:33) 22.44 at async Resolver.resolveImport (/workspace/contracts/node_modules/hardhat/src/internal/solidity/resolver.ts:194:24) 22.44 at async /workspace/contracts/node_modules/hardhat/src/internal/solidity/dependencyGraph.ts:188:28 22.44 at async Promise.all (index 3) 22.44 at async DependencyGraph._addDependenciesFrom (/workspace/contracts/node_modules/hardhat/src/internal/solidity/dependencyGraph.ts:186:5) 22.44 at async Promise.all (index 17) 22.44 at async Function.createFromResolvedFiles (/workspace/contracts/node_modules/hardhat/src/internal/solidity/dependencyGraph.ts:15:5) 22.44 at async Environment._runTaskDefinition (/workspace/contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:333:14) 22.44 at async Environment.run (/workspace/contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:166:14) 22.44 22.44 Caused by: Error: Cannot find module 'eigenda/package.json' from '/workspace/contracts' 22.44 at Function.resolveSync [as sync] (/workspace/contracts/node_modules/hardhat/node_modules/resolve/lib/sync.js:89:15) 22.44 at Resolver._resolveNodeModulesFileFromProjectRoot (/workspace/contracts/node_modules/hardhat/src/internal/solidity/resolver.ts:457:20) 22.44 at Resolver._resolveLibrarySourceName (/workspace/contracts/node_modules/hardhat/src/internal/solidity/resolver.ts:311:30) 22.44 at Resolver.resolveSourceName (/workspace/contracts/node_modules/hardhat/src/internal/solidity/resolver.ts:103:33) 22.44 at async Resolver.resolveImport (/workspace/contracts/node_modules/hardhat/src/internal/solidity/resolver.ts:194:24) 22.44 at async /workspace/contracts/node_modules/hardhat/src/internal/solidity/dependencyGraph.ts:188:28 22.44 at async Promise.all (index 3) 22.44 at async DependencyGraph._addDependenciesFrom (/workspace/contracts/node_modules/hardhat/src/internal/solidity/dependencyGraph.ts:186:5) 22.44 at async Promise.all (index 17) 22.44 at async Function.createFromResolvedFiles (/workspace/contracts/node_modules/hardhat/src/internal/solidity/dependencyGraph.ts:15:5) 22.47 22.64 error Command failed with exit code 1. 22.65 info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. 22.69 make: *** [Makefile:336: .make/solidity] Error 1

afkbyte added 2 commits April 6, 2024 14:50

add daProviderForEigenDA type that conforms to DataAvailabilityProvid…

dc129e6

…er interface

update inbox functions containing references to eigenDAReader and main

8dee1fd

afkbyte requested a review from teddyknox April 6, 2024 19:44

afkbyte added 2 commits April 17, 2024 14:25

ported over sequencer code

f01b272

remove test file

8d5742b

ethenotethan reviewed Apr 26, 2024

View reviewed changes

ethenotethan reviewed May 6, 2024

View reviewed changes

prpeh reviewed Jun 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Conform to nitro's DataAvailabilityProvider interface #1

Conform to nitro's DataAvailabilityProvider interface #1

Uh oh!

afkbyte commented Apr 6, 2024

Uh oh!

ethenotethan left a comment

Uh oh!

ethenotethan Apr 25, 2024

Uh oh!

ethenotethan Apr 25, 2024

Uh oh!

ethenotethan Apr 25, 2024

Uh oh!

ethenotethan Apr 26, 2024

Uh oh!

ethenotethan Apr 26, 2024

Uh oh!

ethenotethan Apr 26, 2024

Uh oh!

ethenotethan Apr 26, 2024

Uh oh!

ethenotethan Apr 26, 2024

Uh oh!

ethenotethan Apr 26, 2024

Uh oh!

ethenotethan Apr 26, 2024

Uh oh!

Tristan-Wilson commented Apr 30, 2024

Uh oh!

ethenotethan May 6, 2024

Uh oh!

prpeh Jun 3, 2024

Uh oh!

prpeh Jun 5, 2024

Uh oh!

Uh oh!

	pointer, err := b.eigenDAWriter.Serialize(blobID)
	sequencerMsg, err = b.eigenDAWriter.Serialize(blobID)

Conform to nitro's DataAvailabilityProvider interface #1

Are you sure you want to change the base?

Conform to nitro's DataAvailabilityProvider interface #1

Uh oh!

Conversation

afkbyte commented Apr 6, 2024

Uh oh!

ethenotethan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tristan-Wilson commented Apr 30, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!